Skip to content

🐛 Fix image properties propagation #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Jan 17, 2025
Merged

🐛 Fix image properties propagation #224

merged 2 commits into from
Jan 17, 2025

Conversation

@matofeder
Copy link
Member

@matofeder matofeder commented Jan 16, 2025 •
edited
Loading

What this PR does / why we need it:

This PR contains a hot-fix that allows CreateOpts.properties definition via node-images.yaml as follows:

apiVersion: openstack.infrastructure.clusterstack.x-k8s.io/v1alpha1
openStackNodeImages:
  - createOpts:
      container_format: bare
      disk_format: qcow2
      name: ubuntu-capi-image-v1.29.9
      visibility: private
      properties:
        hw_disk_bus: "scsi"
        hw_rng_model: "virtio"
        hw_scsi_model: "virtio-scsi"
    url: ...

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #223

Special notes for your reviewer:

TODOs:

  • squash commits
  • include documentation
  • add unit tests

@matofeder matofeder force-pushed the fix_image_properties branch from 115c18c to afeee9f Compare January 16, 2025 21:42
@matofeder matofeder changed the title Fix image properties propagation 🐛 Fix image properties propagation Jan 16, 2025
@matofeder matofeder force-pushed the fix_image_properties branch from afeee9f to 4bd0465 Compare January 16, 2025 21:56
@matofeder
Update unittests
3f53487 
Signed-off-by: Matej Feder <[email protected]>
@matofeder matofeder force-pushed the fix_image_properties branch from 43c746c to 3f53487 Compare January 16, 2025 23:03
chess-knight
chess-knight approved these changes Jan 17, 2025
View reviewed changes
Copy link
Member

@chess-knight chess-knight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jschoone jschoone requested a review from Nils98Ar January 17, 2025 10:33
Nils98Ar
Nils98Ar reviewed Jan 17, 2025
View reviewed changes
Copy link
Member

@Nils98Ar Nils98Ar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, looks good!

Is there a reason that hw_disk_bus the only property that you set in the unit tests?

@matofeder
Copy link
Member Author

matofeder commented Jan 17, 2025

Is there a reason that hw_disk_bus the only property that you set in the unit tests?

There is no reason, I just arbitrarily picked some.

jschoone
jschoone approved these changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@jschoone jschoone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing it that fast @matofeder.
I can also confirm it works:

openstack image show ubuntu-capi-image-v1.31.4 -c properties -f json
{
  "properties": {
    ...
    "hw_disk_bus": "scsi",
    "hw_rng_model": "virtio",
    "hw_scsi_model": "virtio-scsi"
  }
}

@jschoone
Copy link
Contributor

jschoone commented Jan 17, 2025

@matofeder can you create a release with this fix?

@matofeder
Copy link
Member Author

matofeder commented Jan 17, 2025

@matofeder can you create a release with this fix?

sure, I will do that asap

@matofeder matofeder merged commit 56aa0f4 into main Jan 17, 2025
11 checks passed
@matofeder matofeder deleted the fix_image_properties branch January 17, 2025 15:30
@Nils98Ar
Copy link
Member

Nils98Ar commented Jan 17, 2025

Thanks @matofeder! I didn't expect this to be solved so quickly :)

@matofeder
Copy link
Member Author

matofeder commented Jan 17, 2025

I tagged the latest main with v0.1.0-alpha.6, I believe that the github pipeline does the rest (release)..

@matofeder
Copy link
Member Author

matofeder commented Jan 17, 2025 •
edited
Loading

OK, I just had to click on the button and publish the release produced by the pipeline.

https://github.com/SovereignCloudStack/cluster-stack-provider-openstack/releases/tag/v0.1.0-alpha.6

let me know if you encounter any issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jschoone jschoone jschoone approved these changes

@Nils98Ar Nils98Ar Nils98Ar approved these changes

@chess-knight chess-knight chess-knight approved these changes

@jklippel jklippel Awaiting requested review from jklippel

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Images miss SCS metadata

5 participants

@matofeder @jschoone @Nils98Ar @chess-knight